Skip to content

Conversation

@zksite
Copy link

@zksite zksite commented Oct 23, 2025

Fix collection existence check broken by error message change in Chroma API.

The Chroma backend recently changed its error message from:
"Collection [...] does not exists"
to:
"Collection [...] does not exist"

This broke the string-based detection in getCollection(), causing 404s to be treated as unexpected errors and preventing initialization.

This PR replaces fragile string matching with reliable HTTP 404 status code checking via HttpClientErrorException.NotFound.

Copilot AI review requested due to automatic review settings October 23, 2025 03:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug in the Chroma vector store initialization where the collection existence check broke due to an upstream API error message change. The fix replaces fragile string-based error detection with robust HTTP 404 status code checking.

Key Changes:

  • Replaced string matching on error messages with HTTP status code checking using HttpClientErrorException.NotFound
  • Added proper exception handling with try-catch block to differentiate between "collection not found" (404) and genuine errors

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

collection = this.chromaApi.getCollection(this.tenantName, this.databaseName, this.collectionName);
}
catch (Exception ex) {
if (!(ex.getCause() instanceof HttpClientErrorException.NotFound)) {
Copy link

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception handling logic checks ex.getCause() for HttpClientErrorException.NotFound, but if chromaApi.getCollection() directly throws HttpClientErrorException.NotFound (not wrapped), this check will fail and re-throw the exception incorrectly. Consider also checking if ex itself is an instance of HttpClientErrorException.NotFound before checking the cause: if (!(ex instanceof HttpClientErrorException.NotFound || ex.getCause() instanceof HttpClientErrorException.NotFound))

Suggested change
if (!(ex.getCause() instanceof HttpClientErrorException.NotFound)) {
if (!(ex instanceof HttpClientErrorException.NotFound || ex.getCause() instanceof HttpClientErrorException.NotFound)) {

Copilot uses AI. Check for mistakes.
Add try-catch block to handle HttpClientErrorException.NotFound when getting collection during initialization. This prevents initialization failures when the collection doesn't exist yet.

Signed-off-by: hanjie <[email protected]>
collection = this.chromaApi.getCollection(this.tenantName, this.databaseName, this.collectionName);
}
catch (Exception ex) {
if (!(ex.getCause() instanceof HttpClientErrorException.NotFound)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this check be part of ChromaApi:233?

If I am not mistaken, the ChromaApi still contains the exception message check.
That exception message check will also still prevent ChromaApi#getCollection() from returning null, when a collection not exists, although returning null seems to be the expected behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I’ve looked into this further and realized the issue is in Spring AI’s own Chroma client implementation.

The error message check on line 233 currently uses:

String.format("Collection [%s] does not exists", collectionName)

However, the actual response from the Chroma server is:

Collection [...] does not exist

Since the Chroma HTTP API returns "does not exist" ,As a result, the null-return path for missing collections is never triggered.

The fix is simply to update the expected message::

if (String.format("Collection [%s] does not exist", collectionName).equals(msg)) {
    return null;
}

I’m happy to submit a quick PR if that would be helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants